Skip to content

Add CI performance benchmark suite#223

Open
Max042004 wants to merge 2 commits into
sysprog21:mainfrom
Max042004:issue-195-recovered
Open

Add CI performance benchmark suite#223
Max042004 wants to merge 2 commits into
sysprog21:mainfrom
Max042004:issue-195-recovered

Conversation

@Max042004

@Max042004 Max042004 commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add a reproducible two-tier performance benchmark suite for detecting
regressions in pull requests.

  • Tier 1 runs pinned lmbench workloads for syscall, process, and
    filesystem latency.
  • Tier 2 measures Python startup, Git status, ripgrep, zstd, and Make
    against a deterministic committed corpus.
  • Results include medians and raw samples in structured JSON.
  • Baseline columns are prepared for elfuse, QEMU-based native Linux,
    and OrbStack.

CI integration

Add a benchmark leg to the self-hosted Apple Silicon runtime matrix.
The leg runs exclusively on the host to avoid interference from sibling
jobs, compares results in report-only mode, and uploads the JSON artifact.

Missing baseline metrics are treated as failures, preventing broken or
timed-out benchmarks from silently passing. Benchmark preparation and
cleanup commands are also bounded by timeouts.

The checked-in baseline has not yet been captured on the designated
self-hosted CI machine. It will be refreshed in a follow-up after the
benchmark leg is deployed and runner variance has been characterized.
Comparison therefore remains report-only for now.

Validation

  • Full cold run: approximately 10m 19s
  • Cached CI-equivalent run: approximately 9m 20s
  • Baseline match exits successfully
  • An injected 2x regression exits non-zero
  • A missing baseline metric exits non-zero
  • Benchmark output contains 9 lmbench and 6 application metrics

Closes #195


Summary by cubic

Adds a two-tier performance benchmark suite with a deterministic workload to catch syscall, process-startup, and app-level regressions. Runs on the self-hosted Apple Silicon runner under an exclusive host lock and posts a sanitized PR comment with benchmark deltas. (Issue #195)

  • New Features

    • tests/bench-suite.sh: Tier 1 lmbench (lat_syscall/lat_proc/lat_fs) and Tier 2 app workloads (git status, rg, make, zstd) with warmups; guest-side timing for qemu-aarch64 via tests/bench-timeit.c; writes medians+samples to build/bench-results*.json.
    • Deterministic tests/bench-corpus for stable Tier 2 inputs; excluded from formatting to preserve baselines. Fixtures fetch app tools and cross-compile pinned lmbench.
    • Baselines in tests/bench-baseline.json for elfuse-aarch64, qemu-aarch64, and orbstack. scripts/bench-compare.py flags regressions (tunable via BENCH_REGRESSION_THRESHOLD) and treats missing metrics as NEW by default; scripts/bench-promote.py refreshes baselines.
    • CI: make bench / make bench-ci; BENCH_ENV selects elfuse-aarch64 (default) | qemu-aarch64 | orbstack | native. Benchmark leg isolates measurements with scripts/ci-host-lock.sh. .github/workflows/bench-report.yml uses scripts/bench-report.py to publish a sanitized advisory PR comment from the uploaded report artifact. Docs in docs/testing.md.
  • Migration

    • Run: make bench (or BENCH_ENV=qemu-aarch64 make bench-ci).
    • Compare: scripts/bench-compare.py --report-only.
    • Refresh baselines: run make bench-ci per env, then scripts/bench-promote.py; commit the updated tests/bench-baseline.json.

Written for commit c2c8cbe. Summary will update on new commits.

Review in cubic

Performance results need a stable workload so source-data changes do
not look like runtime regressions. Add a small committed project split
across core, filesystem, and network modules for repeatable Git,
ripgrep, zstd, and Make measurements.

Keep the corpus separate because its contents define the benchmark and
should change only when baselines are deliberately regenerated.
cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

@Max042004
Max042004 force-pushed the issue-195-recovered branch 2 times, most recently from c606264 to 899d081 Compare July 18, 2026 16:11
Comment thread tests/bench-corpus/README.md Outdated
@Max042004
Max042004 force-pushed the issue-195-recovered branch from 899d081 to 3a0517a Compare July 18, 2026 17:18
jserv

This comment was marked as resolved.

@jserv
jserv requested a review from henrybear327 July 19, 2026 07:12
@Max042004
Max042004 force-pushed the issue-195-recovered branch from 3a0517a to 653c5fd Compare July 19, 2026 09:36
@jserv

jserv commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Once the benchmark is complete, upload the results to GitHub Pages, as rv32emu-bench does.

@Max042004
Max042004 force-pushed the issue-195-recovered branch from 653c5fd to fb40607 Compare July 19, 2026 11:59
Comment thread .github/workflows/bench-report.yml Outdated
Comment thread tests/bench-corpus/README.md Outdated
Comment thread tests/bench-corpus/README.md Outdated
@Max042004
Max042004 force-pushed the issue-195-recovered branch from fb40607 to c5495eb Compare July 19, 2026 13:40

@henrybear327 henrybear327 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, LGTM. Just some nits, as the text / comment is bloating a bit I feel.

Thanks.

Comment thread .github/workflows/main.yml Outdated
Comment thread .github/workflows/main.yml Outdated
Comment thread docs/testing.md
green `make check` covers BusyBox validation. Use `make test-busybox` to
iterate on a single applet failure without rerunning the unit suite.

## Performance Benchmarks

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: make the text concise

I think the current testing.md has grown to the size that's already too big and too complicated to read

@jserv

jserv commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Once the benchmark is complete, upload the results to GitHub Pages, as rv32emu-bench does.

I create elfuse-bench and @Max042004 can maintain it directly.

Functional tests do not reveal changes in syscall, process-startup, or
application-level performance. Add a two-tier suite so pull requests
produce repeatable measurements and expose regressions before merge.

Run pinned lmbench microbenchmarks and representative developer
workloads with warmups and median reporting. Store raw samples in JSON,
compare them with captured elfuse, QEMU Linux, and OrbStack baselines,
and treat missing baseline metrics as failures.

Run the suite exclusively on the self-hosted Apple Silicon machine to
avoid interference from sibling jobs. Keep comparison report-only until
runner variance is characterized, while retaining artifacts for review
and baseline refreshes.

Closes sysprog21#195
@Max042004
Max042004 force-pushed the issue-195-recovered branch from c5495eb to c2c8cbe Compare July 20, 2026 16:09
@jserv

jserv commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Instead of using hard-to-understand filenames such as mod_00.c, use more descriptive names. If the files need to be ordered, filenames such as 01-function.c would work well instead.

@jserv jserv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the CI benchmark suite. Findings are inline; the substantive risk is the host-lock lifecycle. The generated tests/bench-corpus/ modules were not line-reviewed.

Checked and found correct: the reader/writer flock logic never over-grants (shared is refused while an exclusive lock is held or pending, and the double writer_pending() check closes the publish race); bench-compare regression math and its baseline<=0 / division guards; threshold parsing; missing-metric-as-hard-failure under --fail-on-missing; bench-promote's atomic tempfile+fsync+os.replace and partial-capture provenance; and the workflow's EXIT-trap releases plus the always() net.

Comment thread scripts/ci-host-lock.sh
if listener is not None:
listener.close()
try:
os.unlink(control_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Control-socket cleanup race that can orphan a live lock holder. The detached holder unconditionally os.unlink(control_path) here even on the clean, socket-driven release path (flock already dropped, released=True). The bench step in main.yml does release (holder A) immediately followed by acquire exclusive (holder B) reusing the same fixed CONTROL_FILE path. If holder A is descheduled -- exactly the busy-host scenario this lock targets -- so its unlink lands after holder B has re-bind()ed the recreated socket, A deletes B's socket. B's later EXIT-trap release then hits FileNotFoundError, reports nothing to release, and B keeps the exclusive flock until the 120-min self-expiry, wedging every sibling leg's shared acquire. Suggest guarding this unlink with if not released: so a successor holder that re-bound the same path is never clobbered.

Comment thread scripts/ci-host-lock.sh
except (FileNotFoundError, ConnectionError, socket.timeout):
pass

for path in (pid_path, control_path):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same class as the holder-side race: the release client unlinks the control socket unconditionally, including when it did not confirm a release. A socket.timeout or ConnectionError on connect/recv is swallowed above, then this loop removes a still-live holder's socket path -- if the holder was briefly stuck before accept(), it never gets the release, never drops its flock, and is now permanently unreachable for any future release (held until self-expiry). Suggest unlinking control_path only when released is true; leave a still-live holder's socket in place so a retry can reach it.

Comment thread tests/bench-suite.sh
prep="$prep && cat /bench-work/corpus/*/*.c > /bench-work/zin"
prep="$prep && i=0 && while [ \$i -lt $ZSTD_DOUBLINGS ]; do cat /bench-work/zin /bench-work/zin > /bench-work/zin.2 && mv /bench-work/zin.2 /bench-work/zin || exit 1; i=\$((i + 1)); done"
qemu_exec timeout "$TEST_TIMEOUT" sh -c "$prep" \
> /dev/null 2>&1 \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The qemu Tier-2 corpus prep (512-shard cp -R plus the zstd doublings) runs under timeout "$TEST_TIMEOUT" -- the per-sample bound -- and bench_dies the whole run on a slow-but-valid guest. The native (prepare_corpus) and orbstack prep paths do the same heavy copy work untimed, so the transports bound prep inconsistently and a legitimately slow guest FS becomes a hard failure rather than a measurement. Suggest bounding prep with its own (larger) timeout applied consistently across transports, separate from the per-sample TEST_TIMEOUT.

Comment thread tests/bench-suite.sh
# Tier-2 rows are bench-timeit's workload-only microseconds; report ms.
apps = data.get("applications", {})
for bench, rows in sorted(apps.items()):
vals = rows[bench]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vals = rows[bench] assumes the metric key equals the bench key. Every current Tier-2 producer emits applications <metric> <metric> <value> so it holds today, but it is an unguarded invariant: any future row where bench != metric raises KeyError and crashes the JSON emitter with an opaque traceback instead of a diagnosable error. Suggest iterating rows.items() (emit each metric under bench) or asserting the invariant with a clear message.

Comment thread tests/bench-timeit.c
return 1;
}

printf("%llu\n", (unsigned long long) (elapsed / 1000));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elapsed / 1000 floors nanoseconds to microseconds, so any workload finishing under 1 microsecond prints 0. Downstream summarize() accepts that as a valid fast sample and bench-compare scores it as a large improvement rather than a broken/instant measurement. Harmless for the current ms-scale corpus, but it removes the only guard against a workload that does no work reading as improved. Suggest emitting higher resolution (keep nanoseconds) or rejecting elapsed < 1000 as an invalid sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI/CD performance benchmarking

3 participants